-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Blocks: Try adding TypeScript type checking and improved JSDoc #16552
Conversation
Just a note: most of the errors reported are the result of one of the following things:
Once those two problem areas are wiped out, you'd probably be able to count the issues on 2 hands. Hope the output is not too daunting! |
I would be curious to know whether all of the syntaxes used here are compatible with Specifically things like:
gutenberg/packages/blocks/src/api/children.js Line 130 in 38cfb2e
However, I would agree with treating this as a separate task. In general, I think it could benefit to pair this effort down to separate pull requests. This one can be a good test ground for getting the work done, then proceeding to extract individual commits for their own pull requests to simplify review (this is what I've done with similar large efforts like #14715, #15226, #3745). |
I'm catching up with your open PRs to see if I didn't miss anything important which would block your efforts. Is #17014 an alternative try to tackle something much simpler first? :)
This one is fixed now and I see that this PR also adds |
Sorry about that. Yeah, this I think was too much for the first PR. We can go ahead and close this for now and I'll come back and cherry pick from this later after the simpler packages are done. 👍 |
Whatever you feel works better for you. The only issue with stale PRs is that you need to rebase them from time to time which might be challenging if the span hundreds of lines :) |
Nah that's fine. I'll either reopen this or redo it later on. You're free to close if you'd like. |
I closed this one, we have enough number of open PRs to track 😅 |
Description
This PR is the result of a week or so of on and off tinkering in the JSDoc and several weeks of submitting PRs to DefinitelyTyped for typescript type definitions.
The gist: This PR adds initial TypeScript compile-only (no-emit) type checking for just the
blocks
package.The goal of this PR is threefold:
go to definitition
in VSCode to see detailed documentation for all types.You'll notice that this PR is not free of errors. Indeed, at the time of writing this, there are 133 reported problems with the
blocks
package. Most of these problems are valid, however a good portion of these are problems with the type definitions which will need to be fixed upstream.If you're using an editor with TypeScript support, all you'd need to do to begin being productive is open up any file within the
blocks
package and you'll see the errors inline.If you don't have an editor with TypeScript support, simply run
npx tsc
anywhere in the project for a print out of all of the errors in your terminal.I probably left out a bunch of info, so please feel free to discuss any concerns/questions/comments freely and I'll do my best to resolve those.
Here is a list of the currently reported errors for the record
TODO items remaining:
valid-jsdoc
and instead useeslint-plugin-jsdoc
for better JSDoc support.@types/wordpress__blocks
for accuracy.How has this been tested?
N/A
Screenshots
N/A
Types of changes
Devops
Checklist:
Of note: The readme parser seems to be broken with the changes I made to the JSDoc for whatever reason. The error message was vague.